refactor: split logging module + cleanup + lifecycle test#99
Merged
Conversation
REF-4: Split lite_bootstrap/instruments/logging_instrument.py (212 lines, four concerns) into: - logging_factory.py: MemoryLoggerFactory, _MemoryLoggerFactoryConfig, _serialize_log_with_orjson_to_string, AddressProtocol, RequestProtocol, ScopeType. Single structlog-conditional gate at module top. - logging_instrument.py: LoggingConfig, LoggingInstrument, tracer_injection. Re-imports the moved public symbols (MemoryLoggerFactory, AddressProtocol, RequestProtocol, ScopeType) for backward compatibility — existing imports from lite_bootstrap.instruments.logging_instrument continue to work. LOW-6: MemoryLoggerFactory.__init__ now takes a single _MemoryLoggerFactoryConfig dataclass instead of four logging-config kwargs. The dataclass is underscore-prefixed (internal); tests import it explicitly when constructing the factory directly. REF-2: Delete the dead `if import_checker.is_structlog_installed and import_checker.is_litestar_installed:` defensive check in LitestarLoggingInstrument.bootstrap. The check is unreachable — the instrument couldn't have been registered without both packages installed. LOW-8: Add a docstring to LoggingInstrument._unset_handlers documenting that the mutation is permanent (teardown does not restore handlers). LOW-9: Add a docstring to LoggingInstrument.teardown documenting that root logger level is unconditionally reset to WARNING. TEST-8: Add test_logging_instrument_lifecycle_replay exercising bootstrap → teardown → bootstrap → teardown and verifying the instrument is functional after the second bootstrap. No external behavior change. Updated factory tests to use the new config-based constructor. Closes REF-2, REF-4, LOW-6, LOW-8, LOW-9, TEST-8 from the audit.
Codecov Report✅ All modified and coverable lines are covered by tests.
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The biggest PR of the deferred-refactors sequence — six logging-area items bundled:
logging_instrument.py(212 lines, four concerns) into a newlogging_factory.py(MemoryLoggerFactory + serializer + protocols + the new internal_MemoryLoggerFactoryConfig) and a slimmerlogging_instrument.py(config + instrument + tracer injection)._MemoryLoggerFactoryConfigand_serialize_log_with_orjson_to_stringlive OUTSIDE the structlog gate sologging_instrument.pycan import them unconditionally;MemoryLoggerFactory(which inherits fromstructlog.stdlib.LoggerFactory) stays inside. Backward compatibility: the moved public symbols are re-imported intologging_instrument.pywith explicit__all__, so existingfrom lite_bootstrap.instruments.logging_instrument import MemoryLoggerFactorycontinues to work.if import_checker.is_structlog_installed and import_checker.is_litestar_installed:check inLitestarLoggingInstrument.bootstrap(). The instrument couldn't have been registered without both packages installed.MemoryLoggerFactory.__init__now takes a singleconfig: _MemoryLoggerFactoryConfigkwarg instead of four logging-config kwargs. Underscore-prefixed (internal); two existing direct-construction tests updated to use the new constructor.LoggingInstrument._unset_handlersdocumenting that the mutation is permanent —teardown()does NOT restore.LoggingInstrument.teardown()documenting that root logger level is unconditionally reset toWARNING.test_logging_instrument_lifecycle_replayexercises bootstrap → teardown → bootstrap → teardown and verifies the instrument is functional after the second bootstrap.No external behavior change. 128/128 tests pass;
ty checkandruffboth clean.Closes REF-2, REF-4, LOW-6, LOW-8, LOW-9, TEST-8 from an internal audit.
Test plan
just test -- tests/instruments/test_logging_instrument.py -v— passes (updated factory tests + new lifecycle replay).just test— 128/128.just lint— clean (no# noqa: PLR2004).logging_instrument.pycover all the public symbols (MemoryLoggerFactory, AddressProtocol, RequestProtocol, ScopeType).logging_factory.py(_MemoryLoggerFactoryConfigand serializer outside the gate; onlyMemoryLoggerFactoryinside) is the right shape.Notes for reviewer
The
_MemoryLoggerFactoryConfigis intentionally underscore-prefixed — it's an internal helper, not new public API. Tests that constructMemoryLoggerFactorydirectly need to import it explicitly. The trade-off was discussed in the plan: extending the public API surface with a non-prefixed name vs accepting that tests reach into the internal config. The latter is consistent with how tests already access other internal-only types.🤖 Generated with Claude Code